Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tree-sitter-htmldjango #3249

Closed
wants to merge 5 commits into from
Closed

Conversation

pickfire
Copy link
Contributor

@pickfire pickfire commented Jul 29, 2022

Great thanks to @interdependence for building tree-sitter-django.

I didn't check why the variables doesn't seem to work but other than that it is already good enough for now.

cc @WindSoilder I added tag and keyword.function to monokai spectrum since I was using it.

@WindSoilder
Copy link
Contributor

Ok, I'll checkout and update other monokai pro theme as soon as possible

name = "htmldjango"
scope = "source.htmldjango"
injection-regex = "htmldjango"
file-types = ["html"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this entry beats regular html for .html files. We have another conflict like this between verilog and V and I think the only way around it for now is to have one of them where you have to override it in a .helix/languages.toml or ~/.config/helix/languages.toml:

[[language]]
name = "html"
file-types = []

[[language]]
name = "htmldjango"
file-types = ["html"]

Since HTML is more popular than django I think we should disable this language by default

Suggested change
file-types = ["html"]
file-types = []

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it beats html, but doesn't it only apply if say the root file manage.py is found?

Copy link
Contributor Author

@pickfire pickfire Aug 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, it doesn't. Maybe we should apply it conditionally based on the roots? I will take some time before I get to work on this later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also under the impression that file-types would only be superceded if manage.py was found based on these settings, a file I've never heard of or seen people using until I did some web searching based on this PR's options. I'll need to re-read those options.

Copy link
Contributor Author

@pickfire pickfire Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now, but I am still a bit not quite happy with the result that the ordering is a bit messy (it will take the first glob that matches the path and have the root within the path). Some parts of it were taken from #2455, but I haven't updated the docs yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now, but I am still a bit not quite happy with the result that the ordering is a bit messy (it will take the first glob that matches the path and have the root within the path). Some parts of it was taken from #2455, but I haven't updated the docs yet.

Comment on lines 1 to 3
(paired_statement) @indent
(end_paired_statement) @indent_end
(branch_statement) @branch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like they're in nvim style (we don't use branch or indent_end iirc). Could you adjust them to the helix ones? https://docs.helix-editor.com/guides/indent.html

I'm not caught up yet on the indentation mechanism so actually I'm not too sure how to these should look myself 😅

The default indent mechanism (removing this file) might work well. It seems to work pretty well for HTML for me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it but didn't go and test it out.

Default to django if it matches file-types of templates/**.html and have root
of manage.py, otherwise defaults to html.
Comment on lines +1 to +2
(paired_statement) @indent
(end_paired_statement) @outdent
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this again, not sure if we want this since vim does not indent django paired statements.

@pickfire pickfire requested a review from the-mikedavis August 18, 2022 00:31
languages.toml Outdated Show resolved Hide resolved
runtime/queries/htmldjango/textobjects.scm Outdated Show resolved Hide resolved
If multiple languages have the same extension or file name, prioritize the one
that have a matched root. Example with django,

1. html extension matched, but can use both language html and htmldjango
2. html have no roots and django have manage.py roots
3. if djangohtml roots are found in root search path, prefer djangohtml
4. otherwise defaults to first match
}
}

pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec<PathBuf> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite know why are we not using Path here, but I don't feel like changing this here.

@kirawi kirawi added the S-waiting-on-review Status: Awaiting review from a maintainer. label Sep 13, 2022
@pickfire
Copy link
Contributor Author

pickfire commented Oct 5, 2022

@the-mikedavis What do you think about the glob changes? If you are okay then I will fix the conflicts.

@the-mikedavis
Copy link
Member

I like the new version where languages can share file-types 👍. I think that will help with v and verilog which share a file-type too.

It could probably use a line in the documentation somewhere (maybe book/src/languages.md?) to make it explicit what the behavior is

@MrConorAE
Copy link

Hi all, any update on this? I'm currently bodging my way through this by copying the .scm files, but it isn't working well. Happy to test with my codebase if that's needed.

@pascalkuthe
Copy link
Member

closing this one as stale, thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants